Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Also log any extra data attached to an error #46

Merged
merged 2 commits into from
Jul 19, 2019
Merged

Conversation

flintinatux
Copy link
Contributor

Screen Shot 2019-07-18 at 5 06 24 PM

I want to fix this problem in Datadog. But currently we clean errors before logging them, and just log the { message, name, stack }. Any injected trace ids would be scrubbed, and errors would not be attached to the corresponding traces.

This PR fixes that, by continuing to log at least the { message, name, stack } for errors, but also any other data that is attached to the error.

@flintinatux flintinatux requested a review from a team July 18, 2019 21:19
@flintinatux flintinatux self-assigned this Jul 18, 2019
lib/logger.js Outdated
pick(['message', 'name', 'stack'])

const flattenProps =
converge(merge, [ identity, clean ])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😕 I don't follow. Are we picking the props off of an object & then merging them back into the same object?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Javascript errors are super weird. The { message, name, stack } properties are not "own" properties. Instead they are built-in getters. Consequently:

JSON.stringify(new Error('my message')) === '{}'

Which is not at all what we want. In the past we've resolved this with a pick, which reads the specified props whether they are "own" or not.

With the code in this PR, if we've attached new "own" properties to an error, it will pick off the special { message, name, stack } props, and then merge them with the extra "own" props, flattening them into a serializable object.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oooh... okay. A comment may be in order.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe "own" is the wrong word. I may be thinking of "enumerable". Those three props are not "enumerable", which is why a JSON.stringify won't pick them up when serializing.

@flintinatux
Copy link
Contributor Author

squirrel

@flintinatux flintinatux merged commit 376f0a1 into master Jul 19, 2019
@flintinatux flintinatux deleted the extra-error-data branch July 19, 2019 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants